Skip to content

Conversation

@cernymi
Copy link

@cernymi cernymi commented Nov 7, 2025

Related Issue

Issue #1479

(and PR #1427 seems missing ALLOWED_QUERY_PARAMS part)

Modified Behavior

New Behavior

None

...

Contrast to Current Behavior

You'll be able to create more services with the same name but different parent. For netbox 4.4.4 and never
Parent param introduced in Netbox 4.3.0

...

Discussion: Benefits and Drawbacks

Services will work correctly again for Netbox 4.4.4 (4.3.0) and never

  • parent_object_type and parent_object_id should be added to services ALLOWED_QUERY_PARAMS
  • be able to query services correctly. Only name of service is not globally unique for Netbox 4.3.0 and never. So parent parameters should be involved.

...

Changes to the Documentation

None

...

Proposed Release Note Entry

Fix netbox_services for Netbox 4.3.0 and never

...

Double Check

  • I have read the comments and followed the CONTRIBUTING.md.
  • I have explained my PR according to the information in the comments or in a linked issue.
  • My PR targets the devel branch.

@cernymi cernymi force-pushed the cernymi/fix-issue-1479 branch from f7c1398 to 47a3c14 Compare November 7, 2025 19:15
- allow compare even 3 digit versions or more
- sanitize inputs - full_version can sometimes be 4.2.9-Docker-3.2.1
  just use only 4.2.9
- allow compare even 3 digit versions or more
- sanitize inputs - full_version can sometimes be 4.2.9-Docker-3.2.1
  just use only 4.2.9
- elif parent == "services":
    is workaround for Netbox 4.3.0 - 4.4.3 - #20554
    GET Parent_object_type wrong data type - integer instead of string
    Just delete parent_object_type and parent_object_id
    GET is broken anyway
- netbox-version can sometimes be 4.2.9-Docker-3.2.1
  but expecting full_version is just 4.2.9
  do not see any purpose Docker-3.2.1
@cernymi cernymi force-pushed the cernymi/fix-issue-1479 branch from 47a3c14 to 5a4514c Compare November 7, 2025 19:23
@sc68cal
Copy link
Contributor

sc68cal commented Nov 13, 2025

I have this on my list to take a look at, sorry for the delay

@sc68cal
Copy link
Contributor

sc68cal commented Nov 14, 2025

So, my main problem with this PR is that there are a couple changes in this PR. They are valid changes and good to have, but I would feel more comfortable if we broke them apart so that if there was a problem, we could easily revert. If that is even possible? I do understand why they are all part of this PR but changes to how we parse the version string from the Netbox API response is always a little fraught. We recently got bitten by a change that the Docker deployment of NetBox does where they append their release version to the string, so that is why I am a little shy about this PR.

If this is indeed an all or nothing change, I would appreciate some unit tests around version string parsing just to give me peace of mind.

Thanks, and great work.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants